Preprocessor symbols for nullable reference types: Define a single nullable context. #1123
Conversation
The first commit ports in the text from dotnet#700 substantially without change. The only editorial change is to move the clause on "Nullable directives" in lexical-structure before the clause on Pragma directives. In addition, it fixes build warnings
Remove an Xref to a section that's not in this PR.
Alternative PR to dotnet#1108. This branch builds on the work in dotnet#1108. This is an alternative approach that defines a single `nullable` context. The nullable context has two properties: `annotations` and `warnings` that can be enabled or disabled separately.
BillWagner
added
the
meeting: discuss
There was a problem hiding this comment.
A couple of language fixes/clarifications required so it describes the same overall semantics as #1108.
I probably agree (yes I'm hedging ;-)) – If a file starts with an enable/disable and never changes it makes little difference. However I can see that when nullable state is enabled & disabled at different points in a file having one context might make that semantic description simpler. |
|
Example of something that might be hard to understand: #nullable disable
string x = null;
string y = "";
#nullable enable
Console.WriteLine(x.ToString());
Console.WriteLine(y.ToString());Should that give a warning for #1124 states: "However, the disabled context means the compiler does not track null state and does not issue nullable related warnings" - so that would suggest no warning. However, (with .NET 8 SDK) it issues a warning: warning CS8602: Dereference of a possibly null reference. |
This commit addresses all online feedback.
I listened to the June meeting. I updated a bit of text, and added Jon's example from the conversation.
|
I updated everything from the last meeting (both comments, and from the in-meeting discussion). This should be ready to merge @jskeet We can discuss at the meeting, or merge before. |
Let's get a final review at the meeting just in case. It would be handy if everyone could review beforehand though. |
This was discussed unfortunately in Nigel's absence due to connectivity issues. We agreed that it would be better to avoid having multiple PRs hanging around and getting stale - the spec is "always inconsistent" in a draft form, as it doesn't accurately describe any specific version of C#. If Nigel wishes to convince us otherwise, we could always revert the commit and open another PR :) We will dismiss Nigel's review in order to merge now. |
jskeet
dismissed
Nigel-Ecma’s stale review
See comment starting "This was discussed unfortunately"
Fixes #1088
Alternative PR to #1108
This branch builds on the work in #1108. This is an alternative approach that defines a single
nullablecontext. The nullable context has two properties:annotationsandwarningsthat can be enabled or disabled separately.